feat: migrate /api/artist/create REST endpoint from Recoup-Chat to recoup-api#115
Conversation
…coup-api Add new GET endpoint for creating artists with query parameters: - validateCreateArtistQuery.ts: Zod validation for name + account_id - createArtistHandler.ts: handler with CORS, validation, and createArtistInDb - app/api/artist/create/route.ts: GET endpoint with OPTIONS for CORS Reuses createArtistInDb from MYC-3923 migration. Tests: 13 new unit tests (473 total) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…-recoup-chatappapiartistcreateroutets-migrate-to-recoup-api
- Add validateCreateArtistBody.ts with Zod schema (name required, account_id and organization_id optional) - Add createArtistPostHandler.ts for POST requests with JSON body - Update app/api/artists/route.ts to include POST handler - Returns 201 on success per REST conventions - Add 16 unit tests for validation and handler Co-Authored-By: Claude Opus 4.5 <[email protected]>
Removed: - app/api/artist/create/route.ts - lib/artists/createArtistHandler.ts - lib/artists/validateCreateArtistQuery.ts - Related test files Replaced by POST /api/artists endpoint Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove accountId parameter, use only request - Get account_id from x-api-key header via getApiKeyAccountId - Support account_id override for org API keys via validateOverrideAccountId - Update tests to mock auth functions Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use single getApiKeyDetails call instead of two separate auth functions - Use canAccessAccount directly for override validation - Reduces duplicate API key hashing and lookup - Add test for invalid API key case Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Missing authorization check for the organization_id parameter allows any API key to add artists to any organization, regardless of access permissions. This is a privilege escalation vulnerability.
View Details
📝 Patch Details
diff --git a/lib/artists/__tests__/createArtistPostHandler.test.ts b/lib/artists/__tests__/createArtistPostHandler.test.ts
index c764c5d..a08b8d2 100644
--- a/lib/artists/__tests__/createArtistPostHandler.test.ts
+++ b/lib/artists/__tests__/createArtistPostHandler.test.ts
@@ -112,28 +112,21 @@ describe("createArtistPostHandler", () => {
expect(response.status).toBe(403);
});
- it("passes organization_id to createArtistInDb", async () => {
- const mockArtist = {
- id: "artist-123",
- account_id: "artist-123",
- name: "Test Artist",
- account_info: [{ image: null }],
- account_socials: [],
- };
- mockCreateArtistInDb.mockResolvedValue(mockArtist);
+ it("returns 403 when personal API key tries to specify organization_id", async () => {
+ // Personal API keys have orgId=null and cannot add artists to organizations
+ mockGetApiKeyDetails.mockResolvedValue({
+ accountId: "personal-account-id",
+ orgId: null,
+ });
const request = createRequest({
name: "Test Artist",
organization_id: "660e8400-e29b-41d4-a716-446655440001",
});
- await createArtistPostHandler(request);
-
- expect(mockCreateArtistInDb).toHaveBeenCalledWith(
- "Test Artist",
- "api-key-account-id",
- "660e8400-e29b-41d4-a716-446655440001",
- );
+ const response = await createArtistPostHandler(request);
+ expect(response.status).toBe(403);
+ expect(mockCreateArtistInDb).not.toHaveBeenCalled();
});
it("returns 401 when API key is missing", async () => {
@@ -206,4 +199,50 @@ describe("createArtistPostHandler", () => {
expect(response.status).toBe(500);
expect(data.error).toBe("Database error");
});
+
+ it("returns 403 when org API key lacks access to organization_id", async () => {
+ mockGetApiKeyDetails.mockResolvedValue({
+ accountId: "550e8400-e29b-41d4-a716-446655440000",
+ orgId: "550e8400-e29b-41d4-a716-446655440000",
+ });
+
+ const request = createRequest({
+ name: "Test Artist",
+ organization_id: "550e8400-e29b-41d4-a716-446655440001",
+ });
+ const response = await createArtistPostHandler(request);
+
+ expect(response.status).toBe(403);
+ expect(mockCreateArtistInDb).not.toHaveBeenCalled();
+ });
+
+ it("allows org API key to add artist to its own organization_id", async () => {
+ const orgId = "550e8400-e29b-41d4-a716-446655440000";
+ mockGetApiKeyDetails.mockResolvedValue({
+ accountId: orgId,
+ orgId: orgId,
+ });
+
+ const mockArtist = {
+ id: "artist-123",
+ account_id: "artist-123",
+ name: "Test Artist",
+ account_info: [{ image: null }],
+ account_socials: [],
+ };
+ mockCreateArtistInDb.mockResolvedValue(mockArtist);
+
+ const request = createRequest({
+ name: "Test Artist",
+ organization_id: orgId,
+ });
+ const response = await createArtistPostHandler(request);
+
+ expect(response.status).toBe(201);
+ expect(mockCreateArtistInDb).toHaveBeenCalledWith(
+ "Test Artist",
+ orgId,
+ orgId,
+ );
+ });
});
diff --git a/lib/artists/createArtistPostHandler.ts b/lib/artists/createArtistPostHandler.ts
index 0d0fa4d..041a311 100644
--- a/lib/artists/createArtistPostHandler.ts
+++ b/lib/artists/createArtistPostHandler.ts
@@ -4,6 +4,7 @@ import { validateCreateArtistBody } from "@/lib/artists/validateCreateArtistBody
import { createArtistInDb } from "@/lib/artists/createArtistInDb";
import { getApiKeyDetails } from "@/lib/keys/getApiKeyDetails";
import { canAccessAccount } from "@/lib/organizations/canAccessAccount";
+import { RECOUP_ORG_ID } from "@/lib/const";
/**
* Handler for POST /api/artists.
@@ -71,6 +72,22 @@ export async function createArtistPostHandler(
accountId = validated.account_id;
}
+ // Validate organization_id if provided
+ if (validated.organization_id) {
+ // Only allow adding to an organization if:
+ // 1. The API key is from RECOUP_ORG_ID (universal admin access), OR
+ // 2. The API key is from the same organization as the target organization
+ const isAdmin = keyDetails.orgId === RECOUP_ORG_ID;
+ const isSameOrg = keyDetails.orgId === validated.organization_id;
+
+ if (!isAdmin && !isSameOrg) {
+ return NextResponse.json(
+ { status: "error", error: "Access denied to specified organization_id" },
+ { status: 403, headers: getCorsHeaders() },
+ );
+ }
+ }
+
try {
const artist = await createArtistInDb(
validated.name,
Analysis
Missing authorization check for organization_id parameter allows privilege escalation
What fails: createArtistPostHandler() in lib/artists/createArtistPostHandler.ts validates the account_id parameter with canAccessAccount() but passes organization_id directly to createArtistInDb() without any authorization check, allowing any API key holder to add artists to any organization.
How to reproduce:
# As API key holder from Organization A:
curl -X POST /api/artists \
-H "x-api-key: $ORG_A_API_KEY" \
-H "Content-Type: application/json" \
-d '{
"name": "Unauthorized Artist",
"organization_id": "ORGANIZATION_B_UUID"
}'Result: Returns 201 Created with the artist successfully added to Organization B, even though the API key holder belongs to Organization A and has no authorization to modify Organization B's artists.
Expected: Should return 403 Forbidden per the authorization model used for account_id - only API keys from RECOUP_ORG_ID (admin) or the target organization itself should be allowed to add artists to that organization.
Root cause: The handler checks authorization for account_id (lines 60-72) using canAccessAccount(), but organization_id is passed directly to createArtistInDb() without similar validation. This violates the principle of least privilege and allows cross-organization modification.
Implementation notes: Fix implements authorization check matching the pattern used for account_id:
- Allows RECOUP_ORG_ID (universal admin)
- Allows API keys from the target organization
- Rejects all other API keys with 403 Forbidden
- Personal API keys (orgId=null) cannot add artists to any organization
- Updated tests verify both denial and allow cases
- validateCreateArtistBody now takes NextRequest and returns Promise<NextResponse | ValidatedCreateArtistRequest> - Validates API key, JSON body parsing, and schema in one place - Handler now only handles business logic (account access check, artist creation) - Updated tests for new async signature Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Validation function now handles all auth/authz: API key, body parsing, schema, and account access
- Returns flat { name, accountId, organizationId } instead of nested structure
- Handler is now purely business logic: validate -> create -> respond
- Updated tests to cover 403 case in validation
Co-Authored-By: Claude Opus 4.5 <[email protected]>
The MCP SDK's extra parameter doesn't have an accountId property. account_id must be provided via the tool args from system prompt context. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
/api/artist/createREST endpoint from Recoup-Chat to recoup-apicreateArtistInDbfrom MYC-3923 migrationTest plan
validateCreateArtistQuerycreateArtistHandler🤖 Generated with Claude Code